-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AI: Extending follow-up #2587
base: master
Are you sure you want to change the base?
AI: Extending follow-up #2587
Conversation
code_samples/ai_actions/src/Command/AddMissingAltTextCommand.php
Outdated
Show resolved
Hide resolved
} | ||
|
||
$id = $this->binaryDataHandler->getIdFromUri($uri); | ||
$file = $this->binaryDataHandler->getContents($id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably some test here to see if a file is loaded or if URI wasn't corresponding to a file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a specific solution in mind?
I can't use PHP's is_file
here (I don't have access to the filepath). The implementation itself will throw BinaryFileNotFoundException
if something goes wrong (https://github.com/ibexa/core/blob/main/src/lib/IO/IOBinarydataHandler/Flysystem.php#L66) - catching and rethrowing it seems like an overkill to me
code_samples/ai_actions/src/Command/AddMissingAltTextCommand.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks globaly good. I would change when/how the image URI correctness is checked.
Preview of modified Markdown: |
code_samples/ change report
|
@adriendupuis image correctness improved in 4c323b9 (PHPStan is passing locally with these changes) |
Target: 4.6, master
Things done:
Previews: